Skip to content

Conversation

@sripberger
Copy link

@sripberger sripberger commented Aug 8, 2019

Not all OPTIONS requests are CORS preflight requests from browsers. This middleware correctly checks to make sure that Access-Control-Request-Method is present before fully treating a request as a preflight-- however, it does so after skipping over any of the usual logic for "actual" requests.

The result is that access control headers will not be set for any OPTIONS requests, even though they will still continue into downstream middlewares like other "actual" requests:

cors/index.js

Lines 101 to 107 in 71c4d00

// If there is no Access-Control-Request-Method header or if parsing failed,
// do not set any additional headers and terminate this set of steps.
// The request is outside the scope of this specification.
if (!ctx.get('Access-Control-Request-Method')) {
// this not preflight request, ignore it
return await next();
}

This blocks any and all cross-origin OPTIONS requests that are not preflights, breaking features in other popular Koa middlewares-- such as the automatic 'allowed methods' middlewares from koa-router-- when accessed cross-origin.

I've fixed the problem by moving this check into the if statement that separates "actual" requests from preflight ones.

I've also removed some unnecessary await keywords, rationale explained here:

https://eslint.org/docs/rules/no-return-await

I can of course live without this second bit if you guys have some reason I'm not aware of for wanting to return await in these scenarios. Any performance change will be far below noticeable. I just figured I might as well, and can roll it back if I have to. 👍

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant